Fix #14949: FP leakNoVarFunctionCall when passing resource to constructor - #8765
Fix #14949: FP leakNoVarFunctionCall when passing resource to constructor#8765aadanen wants to merge 4 commits into
Conversation
|
clang-tidy/build fails with sanitizers/build fails with a timeout. As I anticipated this solution is too slow. I will think about how to make it faster. |
It does that all the time, I'll rerun it later... |
| } | ||
| if (bail) | ||
| continue; | ||
| const Token* typeTok = arg->next(); |
There was a problem hiding this comment.
I'm not so sure typeTok is meaningful if it's not preceded by new. I believe in the test cases it's just a (. If we want to bail out here I think it's enough to determine that it's passed to a constructor.
There was a problem hiding this comment.
I don't understand. Using git blame I traced those lines back to 23deadb370 and 54b9fa5523. It seems to me like they are there for a reason. Maybe they are unnecessary, but I feel that should be a separate PR.
Edit: nevermind sorry I see the issue. I will see about reducing these checks.
There was a problem hiding this comment.
I tried removing different subsets of the checks and something like
if (tok->function() && !tok->function()->isConstructor())
continue;
based on your suggestion about checking if it's passed to a constructor but wasn't able to pass all test cases. I've got to do some other work so likely won't be able to solve this today. It might be a bit beyond my limited knowledge as most of my cppcheck hacking has been focused on the front end.
Remove Variable and AllocType type checking when searching for unreleased arguments and when checking if member variables are deallocated in destructor.
This method is a little heavy handed and might hurt performance, but I am unsure how we could be more precise. It would be nice if we could check "If var.isResource() or var.needsToBeDeallocated()" but that would be a different PR i think.